perf: token details performance tweaking v1#30214
Conversation
there are places that do not destructure this object so passing around causes renders on components and hooks
will replace the in-hook computations that require complex state maangement. Use reselect memoisation
this was really imperformant, and made it hard to tease out performance insights. Now the hooks are atomic - easier to test, debug, and reduces scope of what components need to consume (they dont need entire return type, only things they need)
This hook had a chain of other hooks and selectors that are incredibly expensive. The full path found was: useSwapBridgeNavigation(...) → useInitialBridgeTokens() → useBalancesByAssetId() → useTokensWithBalance() (biggest cost) I was not able to improve performance of `useTokensWithBalance`, but instead cleaned up the `useSwapBridgeNavigation(...) → useInitialBridgeTokens()` link - as navigation does not need the complex state computations. Created an isolated `useFetchPopularTokens` which allows the navigation to be decoupled from the rest of the expensive logic. NOTE - we need to sync with the bridge team to see if the useTokensWithBalance is needed.
Prithpal-Sooriya
added
the
area-performance
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
| const enabledChainRanking = useSelector(selectAllowedChainRanking); | ||
|
|
||
| const fetchPopularTokens = useFetchPopularTokens(); | ||
| const prefetchPopularTokens = useCallback(() => { | ||
| if (!enabledChainRanking?.length) { | ||
| return; | ||
| } | ||
| fetchPopularTokens({ | ||
| chainIds: enabledChainRanking.map( | ||
| (chain: { chainId: CaipChainId }) => chain.chainId, | ||
| ), | ||
| }).catch(() => undefined); | ||
| }, [enabledChainRanking, fetchPopularTokens]); |
There was a problem hiding this comment.
fetching tokens have been decoupled from the really expensive useInitialBridgeTokens() hook.
Navigation does not need all the data coming from that expensive hook, it only needs a fetch method (we are prefetching before navigating).
| * Lightweight fetcher hook for the Bridge `/getTokens/popular` endpoint. | ||
| * @returns A callback that performs the cached fetch for the supplied | ||
| */ | ||
| export const useFetchPopularTokens = () => { |
There was a problem hiding this comment.
Hook logic is a lift from the useInitialBridgeTokens hook.
| [includeAssetsObject], | ||
| ); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
This logic is decoupled and placed into useFetchPopularTokens hook mentioned above.
| const { onBuy, onSwap, hasEligibleSwapTokens, networkModal } = | ||
| useStickyTokenActions({ | ||
| token, | ||
| currentTokenBalance, | ||
| sourcePage, | ||
| }); |
There was a problem hiding this comment.
Atomised the god hook useTokenActions, now we have composability and only use return values we need.
| }; | ||
| }; | ||
|
|
||
| export default useStickyTokenActions; |
There was a problem hiding this comment.
sticky actions hooks is a very thin wrapper that composes the different atomic handlers.
There was a problem hiding this comment.
the page action hooks is now a very thin wrapper that composes the different atomic handlers.
| * 2. Native token with highest USD value across other chains | ||
| * 3. Fallback: highest USD-value token across any chain | ||
| */ | ||
| export const computeBuySourceToken = ( |
There was a problem hiding this comment.
note for self - exported so other teams can leverage (the "smart defaults" are a really nice win and can be used on other swap navigation flows)
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
atomic action hooks. Mostly gutted the existing logic from useTokenActions god hook.
Some tweaks for performance.
- instead of
useSelectors at top level in hooks, we can actually dynamically pick state inside callbacks. Keeps useCallback references for longer. Useful to prevent callback re-renders
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d215889. Configure here.
Co-authored-by: Prithpal Sooriya <[email protected]>
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsE2E Test Selection:
Per tag descriptions: SmokeSwap → also select SmokeConfirmations (transaction confirmations are part of the flow). SmokeMoney → also select SmokeWalletPlatform when changes touch wallet home or actions entry to buy/sell. Performance Test Selection: |
|
Apparently there are some unneeded assertions. Worth removing them if true. |
|
@bergarces good callout, these asserts came from mostly unchanged code that I gutted from the original god hook. |
metamaskbotv2
Bot
added
the
release-7.79.0
4 participants
Description
Audited the token details page.
This was not due to the chart - the chart is buttery smooth!
Its due to our state update (we have a few things, even when idle, that produce state updates) and poor selector + hook design.
Biggest culprit to slow down was the useSwapBridgeNavigation - yes... a navigation management file 😄
Changes:
useSwapBridgeNavigationflow, this indirectly consumed some really heavy computations that were never used for the navigation process. More details onuseSwapBridgeNavigationaudit below - we may need a sync with bridge team to optimise.`useSwapBridgeNavigation` audit summary
The hook itself looks small, but it transitively pulls in very heavy hook subtrees that all run on every redux state change.
Previous flow:
useSwapBridgeNavigation(...) →
useInitialBridgeTokens() →
useBalancesByAssetId() →
useTokensWithBalance() (biggest cost)
useTokensWithBalancecontains a useMemo which is practically useless, it takes in many dependencies, all of which are subject to frequent changes. The logic contain in the useMemo also is not cheap - many bigNumber.js calculations on a loop of tokens.Worst part is the navigation flow never even uses this data, waste of CPU cycles.
Audit steps going forward:
Changelog
CHANGELOG entry: perf: token details performance tweaking
Related issues
Fixes: N/A
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Performance checks (if applicable)
trace()for usage andaddTokenfor an exampleFor performance guidelines and tooling, see the Performance Guide.
Pre-merge reviewer checklist
Note
Medium Risk
Refactors Token Details action handling and bridge-token prefetching, which can subtly affect navigation/analytics and swap routing logic. Changes are well-covered by new unit tests but touch user-facing flows.
Overview
Improves Token Details and swap/bridge navigation performance by removing heavy, always-on computations from common hooks and shifting them to lightweight selectors/callbacks.
Introduces
useFetchPopularTokensas a standalone cached fetcher for BridgegetTokens/popular, and updatesuseInitialBridgeTokens+useSwapBridgeNavigationto use it (prefetch now usesselectAllowedChainRankingand avoids pulling in balance-heavy hook trees).Splits Token Details actions into atomic hooks in
useTokenAtomicActions(buy/send/receive/swap) and composes them via a newuseStickyTokenActionsfor the sticky footer;useTokenActionsis simplified to page actions only. AddsselectHasEligibleSwapSourceselector for a cheap swap-eligibility check, and memoizes the return value ofuseCurrentNetworkInfoto reduce churn. Extensive tests are added/updated to match the new hook boundaries and behaviors.Reviewed by Cursor Bugbot for commit 38d4371. Bugbot is set up for automated code reviews on this repo. Configure here.